Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref(node): use native path module to infer path #7415

Merged
merged 1 commit into from
Mar 20, 2023

Conversation

JonasBa
Copy link
Member

@JonasBa JonasBa commented Mar 10, 2023

When I initially profiled the code I noticed that the regexp used in splitPath function was dominating the profiles. Both basename and dirname from @sentry/utils were actually calling splitPath which executed the regexp - there were 3 call sites to basename and dirname, 1/3 was conditional. The "optimization" is to call path.parse once and extract each path component - this seems to have contributed to the largest win in terms of performance

@github-actions
Copy link
Contributor

github-actions bot commented Mar 10, 2023

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 20.48 KB (0%)
@sentry/browser - ES5 CDN Bundle (minified) 63.49 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 19.05 KB (-0.02% 🔽)
@sentry/browser - ES6 CDN Bundle (minified) 56.39 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 20.7 KB (0%)
@sentry/browser - Webpack (minified) 67.61 KB (0%)
@sentry/react - Webpack (gzipped + minified) 20.73 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 52.19 KB (0%)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 33.76 KB (0%)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 26.08 KB (0%)
@sentry/replay ES6 CDN Bundle (gzipped + minified) 43.87 KB (0%)
@sentry/replay - Webpack (gzipped + minified) 37.95 KB (0%)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 62.06 KB (0%)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 55.12 KB (-0.01% 🔽)

@JonasBa
Copy link
Member Author

JonasBa commented Mar 10, 2023

@timfish do you mind pulling the repo and running the test? I'm actually seeing an error locally and I have no idea why/how 🤔

@timfish
Copy link
Collaborator

timfish commented Mar 10, 2023

All the tests pass my end!

@JonasBa JonasBa force-pushed the jb/ref/module-path-normalize branch 3 times, most recently from d3f4399 to c025921 Compare March 16, 2023 16:50
@AbhiPrasad AbhiPrasad changed the title ref(parser): use native path module to infer path ref(node): use native path module to infer path Mar 20, 2023
@AbhiPrasad AbhiPrasad merged commit 8e136c6 into develop Mar 20, 2023
@AbhiPrasad AbhiPrasad deleted the jb/ref/module-path-normalize branch March 20, 2023 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants